Skip to content

fix(github): use different graphql path in enterprise version#7541

Merged
klesh merged 2 commits into
apache:mainfrom
carocad:fix/ghe
Jun 4, 2024
Merged

fix(github): use different graphql path in enterprise version#7541
klesh merged 2 commits into
apache:mainfrom
carocad:fix/ghe

Conversation

@carocad
Copy link
Copy Markdown
Contributor

@carocad carocad commented May 29, 2024

⚠️ Pre Checklist

Please complete ALL items in this checklist, and remove before submitting

  • I have read through the Contributing Documentation.
  • I have added relevant tests.
  • I have added relevant documentation.
  • I will add labels to the PR, such as pr-type/bug-fix, pr-type/feature-development, etc.

Summary

  • Use a different graphql path depending on the use of github.com and github enterprise.

Does this close any open issues?

Other Information

@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. component/plugins This issue or PR relates to plugins pr-type/bug-fix This PR fixes a bug severity/p1 This bug affects functionality or significantly affect ux labels May 29, 2024
endpoint.Path = "/graphql" // see https://docs.github.com/en/graphql/guides/forming-calls-with-graphql
if endpoint.Hostname() != "api.github.com" {
// see https://docs.github.com/en/enterprise-server@3.11/graphql/guides/forming-calls-with-graphql
endpoint.Path = "/api/graphql"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall.
I just want to make sure that all On-premise GitHub deployments are Enterprise Edition? Or in other words, do they all follow this URL-forming pattern?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @klesh ,
Is the question directed to me or you mean you are checking it in the background?
From my side it is hard to know if all on-premise instances use this pattern as I only know the instance we use. Besides from that I double checked with Renovate source code and they also have custom handling for Github Enterprise, see https://github.com/renovatebot/renovate/blob/main/lib/util/github/graphql/datasource-fetcher.ts#L85.

So I would consider it safe to assume that this is the standard for on-premise instances.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the information, very informative.

Copy link
Copy Markdown
Contributor

@klesh klesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 4, 2024
@klesh klesh merged commit 288c01e into apache:main Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/plugins This issue or PR relates to plugins lgtm This PR has been approved by a maintainer needs-cherrypick-v1.0 pr-type/bug-fix This PR fixes a bug severity/p1 This bug affects functionality or significantly affect ux size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][Github] Github deployments without Github Actions

3 participants